Repository navigation
Conversation
The blob gate (blob_gate::decide/blob_events) decided fetches and observes from a reference index that had no producer, so it was never wired into a serving path and blobs were served to any peer. - blob_gate: build_reference_index (with transitive HashSeq children), hashseq_children, fold_group_events (group log -> Worlds, world key = group create-hash hex), RefreshingWorlds (producer-swappable fold). - event_store: EventStore::document_references — applied, unremoved document.put events as (blob hash, group) pairs; LocalEvents exposes it. - serve: when the event store is open (web UI), BlobsProtocol runs the gate handler (push still decided by the AccessPolicy); a refresh task re-folds on the group-warning cadence so removals revoke fetch. Without the store, serve keeps push-only handling and warns the gate is inactive. - local: LocalNode::set_blob_gate lets an embedder supply the gate to serve(); default behavior unchanged. - groups::worlds::Worlds::empty for fail-closed fallbacks. Tests: blob_gate producer tests (index + HashSeq attribution, group-log fold gating, refresh revoking a removed member); existing suites pass.
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 33 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f62c344f2e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| by_world.entry(key).or_default().push(event.clone()); | ||
| } | ||
| Worlds::fold(&by_world, BTreeMap::new()) |
There was a problem hiding this comment.
When the log contains more than one group, this partition produces incomplete event graphs. GroupLog::record sets after to the heads of the entire log (groups/log.rs:123–128), so creating group B after group A makes B's create depend on an event excluded from B's partition. groups::fold::index rejects that partition with MissingDependency, and build_blob_gate consequently replaces all worlds with an empty fold, denying even authorized members every fetch. Preserve the global causal graph while deriving per-group authority, rather than passing dependency-incomplete partitions to Worlds::fold.
Useful? React with 👍 / 👎.
| let Ok(bytes) = store.get_bytes(hash).await else { | ||
| continue; | ||
| }; | ||
| let kids = crate::blob_gate::hashseq_children(&bytes); |
There was a problem hiding this comment.
For a raw document whose bytes contain the 32-byte digest of another stored blob, this treats the document as a collection and grants its world access to that other blob. DocumentRef carries only media type, content hash and size; it does not designate a HashSeq. For example, sharing a binary checksum document in world B can therefore expose the contents of a private world-A blob to B's members: the all_present check succeeds precisely because the private blob is stored locally. Only expand explicitly identified collections, rather than interpreting arbitrary document contents as authorization references.
Useful? React with 👍 / 👎.
| if !seen.insert(hash) { | ||
| continue; | ||
| } | ||
| let Ok(bytes) = store.get_bytes(hash).await else { |
There was a problem hiding this comment.
When a referenced document is large, this loads its entire contents into memory merely to determine whether it might be a HashSeq—even for ordinary raw files. Document sizes are unrestricted, and this traversal runs both before the router starts and every 60 seconds, so a sufficiently large stored document can exhaust memory during startup or kill a running server during refresh. Determine collection status from explicit metadata and use bounded reads for collection expansion instead of materializing every referenced blob.
Useful? React with 👍 / 👎.
| Some(gate) => BlobsProtocol::new( | ||
| &store_handle, | ||
| Some(crate::blob_gate::blob_events( | ||
| Arc::clone(gate), | ||
| crate::blob_gate::BlobPolicy::default(), |
There was a problem hiding this comment.
When serve enables both the web UI and worlds, this gate also intercepts the blob downloads required by the existing world-mirroring flow. ServeWorlds::open_world creates a RecordsStore using the same Docs instance and blob store; RecordsStore::reconcile writes record values with doc.set_bytes, not as event-store document.put events. Those value hashes therefore never enter this gate's reference index, so even an authorized group member receiving a valid records ticket cannot download them. WorldCommand::Mirror calls world_records::replicate, which waits for those blobs and eventually times out for a nonempty records document. Include records blobs and their authorization in the gate before applying it to the shared provider.
Useful? React with 👍 / 👎.
Implements #173 (queue task 2956): the blob gate decided fetches/observes from a reference index that had no producer, so it was never wired into a serving path and blobs were served to any peer.
What changed
blob_gate:build_reference_index(transitive HashSeq children attribution),hashseq_children,fold_group_events(group log →Worlds; world key = group create-hash hex, matching the document events'groupextension),RefreshingWorlds(producer-swappable fold behind theArcthe event handler holds).event_store:EventStore::document_references— applieddocument.putevents the author has not removed, as (blob hash, group) pairs; exposed viaLocalEvents.serve: when the node's event store is open (the web UI path, the only time serve opens it),BlobsProtocolruns the gate handler — fetches/observes membership-checked, pushes still decided by the existingAccessPolicy. A refresh task re-folds on the group-warning cadence (60s), so a removed member loses fetch without a restart. HashSeq expansion in serve only counts a blob as a collection when all its parsed children are present in the store. Without the event store, serve keeps the previous push-only handler and logs a startup warning that the gate is inactive.local:LocalNode::set_blob_gate— the embedder supplies the gate toserve()(LocalNode holds no group log/event store of its own); default behavior unchanged.groups::worlds::Worlds::emptyfor fail-closed fallbacks.Explicit decisions (from the decomposition note on #173)
commands/world.rsReplicaand theworld_records.rstest helper are not serving paths; left ungated, as audited.Tests
blob_gate::producer_tests: index + transitive HashSeq attribution; HashSeq parsing; group-log fold gating a fetch; refresh revoking a removed member.cargo test -p id --lib blob_gate— 16 passed. Neighboring suites (event_store, documents, local_events, groups, protocol) — 250 passed.BlobsProtocol, and gating non-web serve (needs serve to open the event store without the web UI — a behavior change beyond this PR). Both are called out as follow-ups on Phase 3: blob gate is not wired into serve, so blobs are still served to any peer #173.Refs #173.
Need help on this PR? Tag
@codesmith-botwith what you need.